Display proper download state in remote results view#1242
Conversation
Before displaying any results for a remote query, ensure that all downloaded results are in memory. This ensures the proper download icon is displayed alongside each NWO.
aeisenberg
force-pushed
the
aeisenberg/analysis-results-on-restart
branch
from
f40af17 to
0383a91
Compare
| } | ||
|
|
||
|
|
||
| public async loadDownloadedArtifacts( |
There was a problem hiding this comment.
Is this really loadDownloadedAnalyses?
There was a problem hiding this comment.
I was thinking that this is about loading the already downloaded analyses into memory.
There was a problem hiding this comment.
Sorry my comment was about the use of 'artifacts' in the name. I think loadDownloadedArtifacts should be loadDownloadedAnalyses
There was a problem hiding this comment.
Ah...yes...Good point. :)
| public async loadDownloadedArtifacts( | ||
| allAnalysesToCheck: AnalysisSummary[] | ||
| ) { | ||
| const allDownloadedAnalyses = await asyncFilter(allAnalysesToCheck, x => this.isDownloadedNotInMemory(x)); |
There was a problem hiding this comment.
loadAnalysesResults checks if an analysis is already in memory and doesn't attempt to download if it is. So we can simplify this to:
const allDownloadedAnalyses = await asyncFilter(allAnalysesToCheck, x => this.isAnalysisDownloaded(x));(Note I've appended the function name with Analysis to match the convention used in the rest of the file.
There was a problem hiding this comment.
Is this a name change only that you are suggesting? If so, would it be better to have isAnalysisDownloadedNotInMemory?
There was a problem hiding this comment.
I'm suggesting removing the part that it checks if the analysis is already in memory, since that is already happening by loadAnalysesResults which is being called by this function. And once we remove that part I think the name can be simplified too.
There was a problem hiding this comment.
So isAnalysisDownloadedNotInMemory changes to:
private async isAnalysisDownloaded(analysis: AnalysisSummary): Promise<boolean> {
return await fs.pathExists(createDownloadPath(this.storagePath, analysis.downloadLink));
}and here the code changes to
const allDownloadedAnalyses = await asyncFilter(allAnalysesToCheck, x => this.isAnalysisDownloaded(x));There was a problem hiding this comment.
Ah...yes. That is simpler.
aeisenberg
force-pushed
the
aeisenberg/analysis-results-on-restart
branch
from
de06ed1 to
8daa92a
Compare
| } | ||
|
|
||
|
|
||
| public async loadDownloadedArtifacts( |
There was a problem hiding this comment.
Sorry my comment was about the use of 'artifacts' in the name. I think loadDownloadedArtifacts should be loadDownloadedAnalyses
And some renaming.
2 participants
Before displaying any results for a remote query, ensure that all
downloaded results are in memory. This ensures the proper download icon
is displayed alongside each NWO.
Checklist
ready-for-doc-reviewlabel there.